fix: Remove Netty as an agent dependency #1206
Conversation
Netty is currently bundled in the ADOT Java Agent JAR, unnecessarily increasing its size by ~7 MB. This PR removes Netty from the ADOT Java Agent dependency list by: 1. Explicitly removing the Netty BOM. 2. Upgrading the AWS SDK to 2.33.11, which addresses the Netty security risk. 3. Replacing the inclusion of all AWS SDK packages with only the specific modules required by the ADOT Java Agent. Tests performed: Local build: ./gradlew build ✅ Unit tests: ./gradlew test ✅ Smoke/contract tests: ./gradlew appsignals-tests:contract-tests:contractTests ✅
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1206 +/- ##
=============================================
- Coverage 85.71% 67.07% -18.64%
- Complexity 19 525 +506
=============================================
Files 3 54 +51
Lines 49 2694 +2645
Branches 5 376 +371
=============================================
+ Hits 42 1807 +1765
- Misses 3 750 +747
- Partials 4 137 +133 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| compileOnly("io.opentelemetry:opentelemetry-exporter-otlp-common") | ||
|
|
||
| // For OtlpAwsExporter SigV4 Authentication | ||
| runtimeOnly("software.amazon.awssdk:sts") |
There was a problem hiding this comment.
This dependency cannot be removed. It is required for customers using the Sigv4 exporters: #1101
There was a problem hiding this comment.
Why the build and Unit Test not fail after removing it? Do you know which API Agent uses?
There was a problem hiding this comment.
This dependency is managed by the AWS SDK and is used internally in the the AWS SDK authentication libraries to retrieve credentials from all sources. This is already well tested by upstream which is why we don't have unit tests here.
To validate this you have to just check one of these paths work:
https://docs.aws.amazon.com/sdkref/latest/guide/feature-assume-role-credentials.html#feature-assume-role-credentials-settings
There was a problem hiding this comment.
I tested this PR with this document. Both trace and log are working.
Exporting collector-less telemetry using AWS Distro for OpenTelemetry (ADOT) SDK
https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch-OTLP-UsingADOT.html
May I ask why you believe the SigV4 projects need STS? Are you trying to obtain an AWS access token from STS? If so, which AWS account, user, or role are you using? I wasn’t able to find the code.
There was a problem hiding this comment.
Thank you, Luke! Yes, SigV4 requires STS. This request comes from users who rely on this feature but are unable to explicitly set or hardcode their credentials in their environment. Instead, these environments use the STS library to automatically retrieve and authenticate credentials on their behalf.
| } | ||
|
|
||
| api("software.amazon.awssdk:auth:2.33.11") | ||
| api("software.amazon.awssdk:aws-core:2.33.11") |
There was a problem hiding this comment.
Does adding aws-core dependency include sts? If it does can ignore my previous comment. Also would you help test the Sigv4 path to ensure that sts still works?
There was a problem hiding this comment.
Do we have any UTs for Sigv4 work? Core does not include STS.
3 participants
Netty is currently bundled in the ADOT Java Agent JAR, unnecessarily increasing its size by ~7 MB.
This PR removes Netty from the ADOT Java Agent dependency list by:
Tests performed:
Local build: ./gradlew build ✅
Unit tests: ./gradlew test ✅
Smoke/contract tests: ./gradlew appsignals-tests:contract-tests:contractTests ✅
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.